+++ This bug was initially created as a clone of Bug #991016 +++ Coverity analysis of source code in js/src has found a Bad bit shift operation, that probably happened recently. ________________________________________________________________________________________________________ *** CID 1225404: Bad bit shift operation (BAD_SHIFT) /js/src/jsnum.cpp: 1759 in js_strtod<unsigned char>(js::ThreadSafeContext *, const T1 *, const T1 *, const T1 **, double *)() 1753 Vector<char, 32> chars(cx); 1754 if (!chars.growByUninitialized(length + 1)) 1755 return false; 1756 1757 size_t i = 0; 1758 for (; i < length; i++) { >>> CID 1225404: Bad bit shift operation (BAD_SHIFT) >>> In expression "s[i] >> 8", right shifting "s[i]" by more than 7 bits always yields zero. The shift amount is 8. 1759 if (s[i] >> 8) 1760 break; 1761 chars[i] = char(s[i]); 1762 } 1763 chars[i] = 0; 1764 The line was added in: https://hg.mozilla.org/mozilla-central/rev/1962fd3aa819 in a changeset by Jan. Jan, any thoughts on how to move forward here? (not sure how bad this is, so setting s-s first.)

(In reply to Gary Kwong [:gkw] [:nth10sd] from comment #0) > Jan, any thoughts on how to move forward here? (not sure how bad this is, so > setting s-s first.) This is harmless. Latin1Char is 8-bit so "s[i] >> 8" is indeed always 0 but that's fine. This function is templated though so we can't remove it: if CharT is jschar the result can be != 0. Gary, is it easy for you to check if changing: if (s[i] >> 8) to: if (jschar(s[i]) >> 8) helps?

Nope, it is not easy for me to do it. dveditz runs occasional (monthly?) Coverity runs and I (even more) occasionally take a look at the results. Harmless -> opening up.

Gary, it is useful to put something like [cid 1234] for Coverity bugs, so we can find the report from the bugzilla bug. And of course, put a link to this bug in the Coverity report.

Oops, never mind, you put it in the name of the bug itself. Great. :)

(In reply to Andrew McCreight [:mccr8] from comment #3) > And of course, put a link to this bug in the Coverity report. Yes, I also did this.

This should silence Coverity. We use jschar but with some basic range analysis the compiler should be able to eliminate the "if (c >> 8)" anyway if CharT is Latin1Char.

Comment on attachment 8465333 [details] [diff] [review] Workaround Review of attachment 8465333 [details] [diff] [review]: ----------------------------------------------------------------- LGTM

(In reply to Jan de Mooij [:jandem] from comment #6) > This should silence Coverity. We use jschar but with some basic range > analysis the compiler should be able to eliminate the "if (c >> 8)" anyway > if CharT is Latin1Char. Alternate solution is to say |if ((c >> 4) >> 4)|